Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #112 by removing artifact type/extension check #113

Conversation

robth
Copy link
Contributor

@robth robth commented Jul 10, 2019

Fixes #112. The extension as returned by the Maven Artifact Type Registry is always used. As a test I'm attaching a test file -- using the Build Helper Maven Plugin -- for which the type (text-file) and extension (txt) don't match. With the PR all tests pass, without it they don't.

@coveralls
Copy link

coveralls commented Jul 10, 2019

Pull Request Test Coverage Report for Build 401

  • 1 of 1 (100.0%) changed or added relevant line in 1 file are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage increased (+0.6%) to 74.079%

Files with Coverage Reduction New Missed Lines %
src/main/java/com/e_gineering/maven/gitflowhelper/MasterPromoteExtension.java 1 96.15%
Totals Coverage Status
Change from base Build 394: 0.6%
Covered Lines: 583
Relevant Lines: 787

💛 - Coveralls

@bvarner
Copy link
Contributor

bvarner commented Jul 10, 2019

Wow. Thank you for this contribution.

This bit of code existed from the very early beginnings of the project, where one it's first uses were on mulesoft applications. The mule packaging type from the mulesoft plugins worked by attaching a .zip file, but didn't properly map that the 'mule' package should produce a .zip, causing issues. The resulting idea was to just use the extension from the attached artifact file rather than the extension defined by the packaging type (which wasn't defined) and log a warning that things weren't configured properly with the packaging plugin being used (in my previous case - mule).

I never modeled a test case for this, because I'd moved on from supporting the mulesoft app efforts at that client, and without a test case existing, forgot about this. :-( It may have been that the master branch builds would have failed for those projects as well -- I don't think those projects ever updated to using this plugin, and if they did, they'll be using a very old version of the plugin and this won't matter anyhow. :-)

So if I'm looking at this PR properly, this should get things 'working' again for you, and for other properly-configured packaging plugins.

Anyone want to take a stab at seeing if a mulesoft package will work with this?

…as the extension as defined by the ArtifactHandler should always prevail.
@robth robth force-pushed the feature/fix-112-rm-artifact-type-ext-check branch from e419d45 to 066f15e Compare July 19, 2019 15:16
@robth
Copy link
Contributor Author

robth commented Jul 19, 2019

It looks like the Artifact Type Registry doesn't always return the right extension. For example the Maven Bundle Plugin offers a packaging type bundle, with extension jar. See its components.xml. Using the ArtifactHandler instead of the Type Registry does return the correct extension, and also works for the other cases (e.g. files attached with the Build Helper Maven Plugin).

@bvarner bvarner merged commit 018a658 into e-gineering:development Dec 19, 2019
@bvarner bvarner mentioned this pull request Dec 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants